Skip to content

Conversation

@maia-s
Copy link
Contributor

@maia-s maia-s commented Nov 1, 2025

Now that SDL supports loading both BMP and PNG files, it's convenient to have functions that can load a surface from either format. This PR adds SDL_LoadSurface and SDL_LoadSurface_IO to facilitate this. (It also adds SDL_IsBMP and SDL_IsPNG, but this PR doesn't expose those functions publicly.)

For context, I'm making a wrapper for SDL that has optional support for SDL_image. If SDL_image isn't enabled, it loads images using core SDL instead. With these functions in SDL, I won't have to add code to detect BMP and PNG magic in my own code to support loading both BMP and PNG without SDL_image.

@slouken
Copy link
Collaborator

slouken commented Nov 1, 2025

I don't think we should add generic LoadSurface() functions, since at that point people will want us to keep adding more formats in the core library.

I'm not opposed to having a function to detect whether a stream is BMP or PNG though. @icculus, thoughts?

@slouken slouken added this to the 3.4.0 milestone Nov 1, 2025
@maia-s
Copy link
Contributor Author

maia-s commented Nov 1, 2025

Ok. Would you like me to add the Is functions to the public headers instead for that, or would you prefer a new combined detection function that returns an enum with the detected type? (The latter would be more efficient and only adds one exported function, but it has a similar risk of people requesting detection of more types)

@slouken
Copy link
Collaborator

slouken commented Nov 2, 2025

Is there a reason you can't just try to load as PNG and if that fails, try to load as BMP?

@slouken slouken self-assigned this Nov 2, 2025
@icculus
Copy link
Collaborator

icculus commented Nov 2, 2025

Well...

A lot of people aren't going to need more than bmp and png (and jpg, which we also handle in SDL). For people that don't use SDL_image for more exotic and/or modern formats, they're all going to write the exact same function to load stuff. Maybe we should just provide it for them.

If they demand more formats later, we can say no. These are here for good reasons (simple loading of uncompressed images, simple loading of compressed images, and dealing with mjpeg cameras). SDL itself needed these and they are the three most obviously useful formats for starter apps. Need more? Use SDL_image.

@maia-s
Copy link
Contributor Author

maia-s commented Nov 2, 2025

Is there a reason you can't just try to load as PNG and if that fails, try to load as BMP?

I could do that, but then if the PNG fails to load, I don't know if it failed because it wasn't a PNG, or because there was some other issue with the file. If it was the latter and I then go on to try loading it as a BMP, and that fails, SDL will set an error message that says the file wasn't a BMP, which would be confusing if it was actually a malformed PNG and PNGs are supposed to be supported by my function. Of course, I can override that error message with something general like "failed to load image", but then I lose more precise errors that SDL may set. And if someone tries to load a JPG, I can't set an error that the format is unsupported without checking the file magic.

@slouken
Copy link
Collaborator

slouken commented Nov 2, 2025

Okay, let me think about this for a bit.

If we do want this function, does it make sense to call it SDL_LoadImage() instead of SDL_LoadSurface()?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants